sanitise keywords query param to prevent IndexError crash and abuse (#140)#172
Open
ahmedk20 wants to merge 1 commit into
Open
sanitise keywords query param to prevent IndexError crash and abuse (#140)#172ahmedk20 wants to merge 1 commit into
ahmedk20 wants to merge 1 commit into
Conversation
- Add _validate_keyword() helper that checks for missing param,
empty string after strip, and length > 200 characters
- Both /<llm_name>/news_keywords and /raw/news_keywords now return
HTTP 400 with a JSON error instead of crashing with IndexError
- Add 4 unit tests covering each validation case
Fixes c2siorg#140
|
Hi @ahmedk20, this was an issue that I raised and it is therefore up to our mentor @hardik1408 to determine who gets assigned the issue. You opening a PR on this issue without assignment is directly going against the protocol that is outlined in the contributions document. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Added a
_validate_keyword()helper inroutes/NewsRoutes.pythat validatesthe
?keywords=query parameter before it reaches the controller or database.Both
/<llm_name>/news_keywordsand/raw/news_keywordsnow use it.Related Issue
Fixes #140
Motivation and Context
The keywords endpoint passed user input directly to Pinecone with zero
validation, causing three problems:
1. Crash —
IndexErroron missing param2. Empty input passes through
3. No length limit
How Has This Been Tested?
Added 4 unit tests in
tests/unitTest.pycovering each validation case:test_keywords_missing_param?keywords=test_keywords_empty_string?keywords=test_keywords_too_longtest_raw_keywords_missing_param/raw/news_keywordsno paramResults:
Types of changes
Checklist